cli: Enhance tooling for checking dependencies #11483

planger · 2022-07-29T10:04:49Z

What it does

Enhances the theia CLI with dedicated checks regarding pulled dependencies as detailed below.

`theia check:theia-version`

Check that all dependencies have been resolved to the same Theia version.

It'll obtain all package workspaces of the monorepo from package.json/workspaces and run through their node_modules as well as the root node_modules in order to collect all dependencies to @theia/** packages and test whether they all have the same version. It'll exclude @theia/monaco-editor-core though, as it follows the version of Monaco, which differs from the common Theia version.

Parameters:

--suppress or -s: Suppress exiting with failure code

`theia check:hoisted`

Check that all dependencies are hoisted.

This command behaves as before, but uses the same enhanced implementation as the command above and below. Also the output is a bit improved with respect to readability in comparison to the current implementation.

`theia check:dependencies`

Check uniqueness of dependency versions or whether they are hoisted.

This command is the basis for the two commands above, but exposes the full configurability to adjust the dependency checks to specific needs.

Parameters:

--workspaces or -w: Glob patterns of workspaces to analyze, relative to cwd. [array] [default: All glob patterns listed in the package.json's workspaces]
--include or -i: Glob pattern of dependencies' package names to be included, e.g. -i "@theia/**". [array] [default: ["**"]]
--exclude or -e: Glob pattern of dependencies' package names to be excluded. [array] [default: None]
--skip-hoisted or -h: Skip checking whether dependencies are hoisted. [boolean] [default: false]
--skip-uniqueness or -u: Skip checking whether all dependencies are resolved to a unique version. [boolean] [default: false]
--skip-single-theia-version or -t: Skip checking whether all @theia/** dependencies are resolved to a single version. [boolean] [default: false]
--suppress or -s: Suppress exiting with failure code. [boolean] [default: false]

See #11482 for the problem this PR aims to address.

Contributed on behalf of STMicroelectronics.

How to test

There are three commands (see above) to test.
For theia check:hoisted it should be tested whether it prints the same amount of information as before. However, the content may look slightly different now:

To test yarn check:theia-version you could proceed as follows:

Check out this PR and build (yarn)
Go to dev-packages/cli and run yarn link
Create a sample Theia application, e.g. with the theia generator, and run yarn link "@theia/cli" in its root
Running yarn && yarn theia check:theia-version should give no errors:
To test different Theia versions of non-overlapping dependencies, you could remove @theia/terminal from the dependencies of browser-app and electron-app, set all their dependencies to 1.28.0, but add @theia/terminal as a dependency with a 1.26.0 in a Theia extension of your project. Note how no @theia/* dependencies are hoisted. However, in the root node_modules you'll find a mix of versions in the @theia/* dependencies (terminal vs the others). Now run yarn && yarn theia check:theia-version again and it should print:
You can now re-add @theia/terminal with version 1.28.0 to browser-app, which will also lead to non-hoisted dependencies and re-run yarn && yarn theia check:theia-version -s (suppress). You'll get now one error (the same error as above), but also a warning that there is a dependency that isn't hoisted. Also note how the exit code differs because of the -s:

To test yarn check:dependencies you could proceed as above, but now play around with the parameters. Implementation-wise it is the same as the two commands above. Try e.g.
yarn theia check:dependencies --workspaces "packages/**" --exclude "rimraf/**" --skip-single-theia-version
which will find non-hoisted dependencies, only the packages of the folder packages, but will not show an error for rimraf.

Don't forget to yarn unlink in dev-packages/cli after testing.

Review checklist

As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

As a reviewer, I agree to behave in accordance with the review guidelines

planger · 2022-08-09T12:25:49Z

Just wanted to ping if someone would be available to give feedback to this proposal? ☺️ Thanks a lot!

planger · 2022-08-22T16:38:21Z

@msujew Did you have a chance to look at this PR? I'm looking forward to your feedback. Thanks a lot!

dev-packages/cli/src/check-dependencies.ts

planger · 2022-08-24T13:31:17Z

@vince-fugnitto @paul-marechal Thanks a lot for your feedback! I pushed an update which includes your suggestions.

vince-fugnitto

I confirm that the changes work better 👍
I'll give @paul-marechal a chance to perform a final review as well.

dev-packages/cli/package.json

paul-marechal · 2022-08-24T15:19:02Z

Using the following package.json

{
  "name": "theia-app-example",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "dependencies": {
    "@theia/core": "^1.28.0",
    "@theia/plugin-ext": "^1.28.0",
    "@theia/plugin-ext-vscode": "^1.28.0"
  },
  "devDependencies": {
    "@theia/cli": "^1.28.0"
  }
}

Running theia check:dependencies seems to find errors that I believe shouldn't be picked up:

Note how these aren't nested dependencies... As a rule of thumb I think we should only consider packages like node_modules/[@package-namespace/]package-name/package.json.

paul-marechal · 2022-08-24T15:33:20Z

Otherwise I tried polluting my example app by adding @theia/[email protected] to force a duplication and it was correctly picked up. I think it should be good to merge once my previous issue is fixed.

eclipse-theia#11482 Contributed on behalf of STMicroelectronics. Change-Id: If442c9a135dc152082ec810713cf8de24ff2b451 Signed-off-by: Philip Langer <[email protected]>

planger · 2022-08-24T16:43:33Z

Thanks again for your feedback!
@paul-marechal You're right, we should filter package.json's deeply nested in dependencies. I've now added these two ignore globs:

`[^@]*/*/**/${PACKAGE_JSON}`, // package.json that isn't at the package root (and not in an @org)
`@*/*/*/**/${PACKAGE_JSON}`, // package.json that isn't at the package root (and in an @org)

I confirmed that the dependencies you mentioned aren't showing up, while other expected ones still show up.
Can you confirm this works for your example too?

paul-marechal

Successfully picks up duplicated Theia package version, this will be helpful!

This will run the Theia version check after yarn install to check whether multiple versions of @theia/* packages are pulled by direct or indirect dependencies. See also eclipse-theia/theia#11483

This will run the Theia version check after yarn install to check whether multiple versions of @theia/* packages are pulled by direct or indirect dependencies. See also eclipse-theia/theia#11483 Contributed on behalf of STMicroelectronics.

msujew self-requested a review August 9, 2022 12:42

paul-marechal self-requested a review August 23, 2022 00:55

vince-fugnitto reviewed Aug 23, 2022

View reviewed changes

dev-packages/cli/src/check-dependencies.ts Show resolved Hide resolved

dev-packages/cli/src/check-dependencies.ts Outdated Show resolved Hide resolved

dev-packages/cli/src/check-dependencies.ts Outdated Show resolved Hide resolved

paul-marechal reviewed Aug 23, 2022

View reviewed changes

dev-packages/cli/src/check-dependencies.ts Outdated Show resolved Hide resolved

planger force-pushed the planger/issues/11482 branch from a44661b to 1be07cc Compare August 24, 2022 13:23

vince-fugnitto reviewed Aug 24, 2022

View reviewed changes

dev-packages/cli/package.json Outdated Show resolved Hide resolved

planger force-pushed the planger/issues/11482 branch from 1be07cc to 94ac974 Compare August 24, 2022 14:54

cli: Enhance tooling for checking dependencies

1eb5747

eclipse-theia#11482 Contributed on behalf of STMicroelectronics. Change-Id: If442c9a135dc152082ec810713cf8de24ff2b451 Signed-off-by: Philip Langer <[email protected]>

planger force-pushed the planger/issues/11482 branch from 94ac974 to 1eb5747 Compare August 24, 2022 16:40

paul-marechal approved these changes Aug 24, 2022

View reviewed changes

vince-fugnitto added the theia-cli issues related to the theia-cli label Aug 24, 2022

martin-fleck-at merged commit 6bd8e74 into eclipse-theia:master Aug 25, 2022

vince-fugnitto added this to the 1.29.0 milestone Aug 25, 2022

planger deleted the planger/issues/11482 branch September 26, 2022 07:58

planger mentioned this pull request Sep 26, 2022

Add theia check:theia-version by default eclipse-theia/generator-theia-extension#156

Merged

vince-fugnitto mentioned this pull request Dec 6, 2022

cli: Enhance theia CLI to identify builds pulling multiple versions of Theia #11482

Closed

gergaudf mentioned this pull request May 26, 2023

cli: add check for detecting multiple versions of the same theia extension #12572

Closed

This was referenced Jun 2, 2023

cli: Add check:theia-extensions command eclipsesource/theia#79

Closed

cli: Add check:theia-extensions command #12596

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: Enhance tooling for checking dependencies #11483

cli: Enhance tooling for checking dependencies #11483

planger commented Jul 29, 2022

planger commented Aug 9, 2022

planger commented Aug 22, 2022

planger commented Aug 24, 2022

vince-fugnitto left a comment

paul-marechal commented Aug 24, 2022 •

edited

Loading

paul-marechal commented Aug 24, 2022

planger commented Aug 24, 2022

paul-marechal left a comment

cli: Enhance tooling for checking dependencies #11483

cli: Enhance tooling for checking dependencies #11483

Conversation

planger commented Jul 29, 2022

What it does

theia check:theia-version

theia check:hoisted

theia check:dependencies

How to test

Review checklist

Reminder for reviewers

planger commented Aug 9, 2022

planger commented Aug 22, 2022

planger commented Aug 24, 2022

vince-fugnitto left a comment

Choose a reason for hiding this comment

paul-marechal commented Aug 24, 2022 • edited Loading

paul-marechal commented Aug 24, 2022

planger commented Aug 24, 2022

paul-marechal left a comment

Choose a reason for hiding this comment

`theia check:theia-version`

`theia check:hoisted`

`theia check:dependencies`

paul-marechal commented Aug 24, 2022 •

edited

Loading